-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Fast path nitpicks #143307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fast path nitpicks #143307
Conversation
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
@@ -34,13 +34,6 @@ pub(crate) fn codegen_select_candidate<'tcx>( | |||
let (infcx, param_env) = tcx.infer_ctxt().ignoring_regions().build_with_typing_env(typing_env); | |||
let mut selcx = SelectionContext::new(&infcx); | |||
|
|||
if sizedness_fast_path(tcx, trait_ref.upcast(tcx)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function shouldn't ever be reached -- we never do Instance::resolve
on Sized
.
@bors2 try @rust-timer queue r? lcnr perhaps |
This comment has been minimized.
This comment has been minimized.
Fast path nitpicks Miscellaneous commits that I didn't really want to fold into anything else. Fixes one theoretical bug with the fast path not considering polarity for `T: !Sized` bounds.
This comment has been minimized.
This comment has been minimized.
predicate.kind().skip_binder() | ||
&& trait_pred.polarity == ty::PredicatePolarity::Positive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has happened multiple times by now 🤔 forgetting the polarity is too damn easy right now 🤔
I can't really think of a good alternative apart from actually splitting ClauseKind::Trait
and ClauseKind::NegTrait
🤔
removing the def_id
etc functions on TraitPredicate
to require trait_pred.trait_ref.def_id()
instead feels really cumbersome and not that helpful 🤔 changing ClauseKind::Trait
to have TraitRef
and Polarity
as separate fields so that we always have to use (_, _)
could work but feels really ugly
anyways, not part of this PR
@rust-timer build 227ec3b |
r=me after getting perf to work ☠️ |
Finished benchmarking commit (227ec3b): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -2.1%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.3%, secondary 0.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 463.343s -> 462.134s (-0.26%) |
@bors r+ |
@bors rollup |
Rollup of 11 pull requests Successful merges: - #142749 (Add methods for converting bool to `Result<(), E>`) - #143288 (Fix `x clean` with a fifo) - #143307 (Fast path nitpicks) - #143346 (update coherence example) - #143356 (use unsigned_abs instead of `abs` on signed int to silence clippy) - #143370 (remove redundant #[must_use]) - #143378 (simplify receivers for some array method calls) - #143380 (Replace kw_span by full span for generic const parameters.) - #143381 (rustdoc: don't treat methods under const impls or traits as const) - #143394 (compiler: Document and reduce `fn provide`s in hir crates) - #143395 (Always use the pure Rust fallback instead of `llvm.{maximum,minimum}`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143307 - compiler-errors:fast-path-nitpicks, r=lcnr Fast path nitpicks Miscellaneous commits that I didn't really want to fold into anything else. Fixes one theoretical bug with the fast path not considering polarity for `T: !Sized` bounds.
Rollup of 11 pull requests Successful merges: - rust-lang/rust#142749 (Add methods for converting bool to `Result<(), E>`) - rust-lang/rust#143288 (Fix `x clean` with a fifo) - rust-lang/rust#143307 (Fast path nitpicks) - rust-lang/rust#143346 (update coherence example) - rust-lang/rust#143356 (use unsigned_abs instead of `abs` on signed int to silence clippy) - rust-lang/rust#143370 (remove redundant #[must_use]) - rust-lang/rust#143378 (simplify receivers for some array method calls) - rust-lang/rust#143380 (Replace kw_span by full span for generic const parameters.) - rust-lang/rust#143381 (rustdoc: don't treat methods under const impls or traits as const) - rust-lang/rust#143394 (compiler: Document and reduce `fn provide`s in hir crates) - rust-lang/rust#143395 (Always use the pure Rust fallback instead of `llvm.{maximum,minimum}`) r? `@ghost` `@rustbot` modify labels: rollup
Miscellaneous commits that I didn't really want to fold into anything else.
Fixes one theoretical bug with the fast path not considering polarity for
T: !Sized
bounds.